Skip to content

vtl2_settings_worker: additional tracing #2147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 22, 2025
Merged

vtl2_settings_worker: additional tracing #2147

merged 2 commits into from
Oct 22, 2025

Conversation

@mattkur
Copy link
Contributor

@mattkur mattkur commented Oct 10, 2025

While debugging some failures to modify VTL2 settings, I noticed that we didn't have explicit logging in some error paths. Add that logging.

@mattkur mattkur requested review from a team and Copilot and removed request for a team October 10, 2025 22:46
@mattkur mattkur requested a review from a team as a code owner October 10, 2025 22:46
Copilot
Copilot AI reviewed Oct 10, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds explicit tracing/logging statements to improve debugging capabilities in the VTL2 settings worker. The changes provide better visibility into error paths that previously lacked explicit logging.

  • Added trace logging for received VTL2 settings
  • Added warning logs for errors during VTL2 configuration preparation and application phases
  • Refactored settings parsing logic to enable intermediate logging

@mattkur mattkur requested a review from a team October 10, 2025 22:48
smalis-msft
smalis-msft approved these changes Oct 10, 2025
View reviewed changes
Copy link
Contributor

@smalis-msft smalis-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the idea is that this error probably gets logged upstream, but this is goodness too, especially if they aren't actually logged upstream.

@mattkur
Copy link
Contributor Author

mattkur commented Oct 10, 2025

I think the idea is that this error probably gets logged upstream, but this is goodness too, especially if they aren't actually logged upstream.

I think so too. But I'm going to hold off merging this one until I fully understand what's going on. I had errors, yet I saw no error logs.

@github-actions
Copy link

At least one Petri test failed.

@mattkur
Copy link
Contributor Author

mattkur commented Oct 22, 2025

I'm going to merge these, since I think the extra errors are goodness. If too noisy, I will take them out. But these are genuine errors, and should be low volume in prod (even if dupes).

The root cause for me not seeing any errors in my tests was the COM3 regression.

@mattkur mattkur merged commit 812dec7 into microsoft:main Oct 22, 2025
29 checks passed
@mattkur mattkur deleted the vtl2-more-logs branch October 22, 2025 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@smalis-msft smalis-msft smalis-msft approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mattkur @smalis-msft